Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 7 pull requests #125824

Merged
merged 22 commits into from
May 31, 2024
Merged

Rollup of 7 pull requests #125824

merged 22 commits into from
May 31, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

amandasystems and others added 22 commits May 28, 2024 13:05
This shunts all the complexity of siphoning off the drop-use facts
into `LivenessResults::add_extra_drop_facts()`, which may or may
not be a good approach.
When a lazy logical operator (`&&` or `||`) occurs outside of an `if`
condition, it normally doesn't have any associated control-flow branch, so we
don't have an existing way to track whether it was true or false.

This patch adds special code to handle this case, by inserting extra MIR blocks
in a diamond shape after evaluating the RHS. This gives us a place to insert
the appropriate marker statements, which can then be given their own counters.
We do this for `&*` and `&mut*` already; might as well do it for raw pointers too.
The coverage-dump tool already needs `rustc_demangle` for its own purposes, so
the amount of extra code needed for a demangle mode is very small.
This avoids the need to build `rust-demangler` when running coverage tests,
since we typically need to build `coverage-dump` anyway.
This appears to be the canonical way to build a tool with the stage 0 compiler.
…g, r=oli-obk

Revert propagation of drop-live information from Polonius

rust-lang#64749 introduced a flow of drop-use data from Polonius to `LivenessResults::add_extra_drop_facts()`, which makes `LivenessResults` agree with Polonius on liveness in the presence of free regions that may be dropped. Later changes accidentally removed this flow. This PR restores it.
…-obk

Apply `x clippy --fix` and `x fmt` on Rustc

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->

Just run `x clippy --fix` and `x fmt`, and remove some changes like `impl Default`.
coverage: Optionally instrument the RHS of lazy logical operators

(This is an updated version of rust-lang#124644 and rust-lang#124402. Fixes rust-lang#124120.)

When `||` or `&&` is used outside of a branching context (such as the condition of an `if`), the rightmost value does not directly influence any branching decision, so branch coverage instrumentation does not treat it as its own true-or-false branch.

That is a correct and useful interpretation of “branch coverage”, but might be undesirable in some contexts, as described at rust-lang#124120. This PR therefore adds a new coverage level `-Zcoverage-options=condition` that behaves like branch coverage, but also adds additional branch instrumentation to the right-hand-side of lazy boolean operators.

---

As discussed at rust-lang#124120 (comment), this is mainly intended as an intermediate step towards fully-featured MC/DC instrumentation. It's likely that we'll eventually want to remove this coverage level (rather than stabilize it), either because it has been incorporated into MC/DC instrumentation, or because it's getting in the way of future MC/DC work. The main appeal of landing it now is so that work on tracking conditions can proceed concurrently with other MC/DC-related work.

````@rustbot```` label +A-code-coverage
…lcnr

Stop using `translate_args` in the new solver

It was unnecessary and also sketchy, since it was doing an out-of-search-graph fulfillment loop. Added a test for the only really minor subtlety of translating args, though not sure if it was being tested before, though I wouldn't be surprised if it wasn't.

r? lcnr
…-obk

Also InstSimplify `&raw*`

We do this for `&*` and `&mut*` already; might as well do it for raw pointers too.

r? mir-opt
…piler-errors

Also resolve the type of constants, even if we already turned it into an error constant

error constants can still have arbitrary types, and in this case it was turned into an error constant because there was an infer var in the *type* not the *const*.

fixes rust-lang#125760
Don't build the `rust-demangler` binary for coverage tests

The coverage-run tests invoke `llvm-cov`, which requires us to specify a command-line demangler that it can use to demangle Rust symbol names.

Historically this used `src/tools/rust-demangler`, which means that we currently build two different command-line tools to help with the coverage tests (`rust-demangler` and `coverage-dump`).

However, it occurred to me that if we add a demangler mode to `coverage-dump` (which is only a handful of lines and no extra dependencies), then we only need to build one helper binary for the coverage tests, and there is no need for tests to build `rust-demangler` at all.

---

Note that the `rust-demangler` binary is separate from the `rustc-demangle` crate (which both `rust-demangler` and `coverage-dump` use as a dependency to do the actual demangling).

---

So the main benefits/motivations here are:
- Slightly faster builds after a fresh checkout or bootstrap bump.
- Making it clear that currently no tests actually need the `rust-demangler` binary, since the coverage tests can use their own tool instead.
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels May 31, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented May 31, 2024

📌 Commit df9cd17 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 31, 2024
@bors
Copy link
Contributor

bors commented May 31, 2024

⌛ Testing commit df9cd17 with merge ada5e2c...

@bors
Copy link
Contributor

bors commented May 31, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing ada5e2c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 31, 2024
@bors bors merged commit ada5e2c into rust-lang:master May 31, 2024
7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 31, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#125652 Revert propagation of drop-live information from Polonius eede364e9abe1c452247e1137ce21d77ac5872ce (link)
#125730 Apply x clippy --fix and x fmt on Rustc 362a93b355950a15f6b6e3c77830a65362e9bd1c (link)
#125756 coverage: Optionally instrument the RHS of lazy logical ope… 9f31206b8c3478ecfa739fa41dcb071cfde5fda8 (link)
#125776 Stop using translate_args in the new solver 6e0c0fef13fd11ccf295f6e94817b810f9d7ca9a (link)
#125796 Also InstSimplify &raw* 0b25619ce8917aa68bb979f14527b76785da8939 (link)
#125807 Also resolve the type of constants, even if we already turn… 110d3143be564c2ba51f446c48f9485b1663ff85 (link)
#125816 Don't build the rust-demangler binary for coverage tests 9dedc63dce241c902f15187cd6ae12f745ea39f7 (link)

previous master: 2a2c29aafa

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ada5e2c): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.7% [0.5%, 1.1%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [-0.4%, 1.1%] 4

Max RSS (memory usage)

Results (primary 2.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
7.2% [1.6%, 15.2%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-5.7% [-6.4%, -5.0%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.1% [-6.4%, 15.2%] 5

Cycles

Results (primary 0.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.9% [0.7%, 1.3%] 4
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.9% [0.7%, 1.3%] 4

Binary size

Results (primary 0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.4%] 24
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.1% [-0.2%, -0.0%] 12
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.0% [-0.2%, 0.4%] 36

Bootstrap: 669.181s -> 672.771s (0.54%)
Artifact size: 318.82 MiB -> 318.83 MiB (0.00%)

@rustbot rustbot added the perf-regression Performance regression. label May 31, 2024
@pnkfelix
Copy link
Member

pnkfelix commented Jun 4, 2024

  • instruction-counts regressed webrender-2022-opt-full, cargo-opt-{incr-patched, full}
  • cycle-counts regressed webrender-2022-opt-{full, incr-full}, cranelift-codegen-opt-incr-full, and clap-opt-incr-patched
  • history view for webrender shows that the cycle-count effect seems real though not quite as pronounced as the original measurements indicate.
  • there are many potential candidates for the cause here in this rollup.
  • not marking as triaged; doing some followup perf runs on individual PR's

@pnkfelix
Copy link
Member

pnkfelix commented Jun 4, 2024

@rust-timer build 110d314

@rust-timer

This comment has been minimized.

@pnkfelix
Copy link
Member

pnkfelix commented Jun 4, 2024

@rust-timer build 0b25619

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (110d314): comparison URL.

Overall result: no relevant changes - no action needed

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.9% [2.9%, 2.9%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.9% [2.9%, 2.9%] 1

Cycles

Results (secondary 2.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.2% [2.2%, 2.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 669.181s -> 667.291s (-0.28%)
Artifact size: 318.82 MiB -> 318.78 MiB (-0.01%)

@lqd
Copy link
Member

lqd commented Jun 4, 2024

We can only queue a build at a time :/

@rust-timer build 0b25619

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0b25619): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.8% [0.6%, 1.2%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.5% [-0.4%, 1.2%] 4

Max RSS (memory usage)

Results (primary 3.1%, secondary 2.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
5.0% [1.1%, 15.6%] 5
Regressions ❌
(secondary)
2.9% [2.9%, 2.9%] 1
Improvements ✅
(primary)
-6.7% [-6.7%, -6.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.1% [-6.7%, 15.6%] 6

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results (primary 0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.4%] 24
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.1% [-0.2%, -0.0%] 11
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.0% [-0.2%, 0.4%] 35

Bootstrap: 669.181s -> 668.149s (-0.15%)
Artifact size: 318.82 MiB -> 318.86 MiB (0.01%)

@matthiaskrgr matthiaskrgr deleted the rollup-eam7i0p branch September 1, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.